Fix the issue with the thumbnail size #10258

jp-tosca · 2024-01-23T18:54:49Z

What this PR does / why we need it:
It was reported on this issue that a thumbnail will be showing the full resolution image on the search/listing of the datasets. During the testing @landreev discovered that there was an issue also with a discrepancy of the resolution of a published vs a draft dataset.

Which issue(s) this PR closes:

Closes #10220

Suggestions on how to test this:
This PR does not address another issue discovered that sometimes a customized thumbnail will not show, this will be investigated on a separate issue. Probably to test be sure to have some datasets with custom thumbnails from a previous installation.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
It should fix the issues caused to the UI reported on #10220

coveralls · 2024-01-23T19:01:24Z

coverage: 20.142%. remained the same
when pulling 108701c on 10220-thumbnail-size-bug
into e9215e3 on develop.

qqmyers

LGTM

landreev

I would prefer to at least figure out what's going on with the 3rd known bug, before making this PR. To check whether it's new in 6.1; and to at least get an idea of how difficult it would be to fix.
I understand that we may want to open a new issue for it, if it's something difficult/time-consuming - but we don't even know that yet.

landreev · 2024-01-23T19:29:37Z

another issue discovered that sometimes a customized thumbnail will not show

Just to be clear, it's not really "sometimes", as in, it's not a mystery of any kind - it appears to be a very clearly/narrowly defined condition: custom logos don't work unless there is at least one image Datafile in the dataset.

landreev · 2024-01-23T19:43:52Z

(meant to add this here:)
It is new as of 6.1. Can be easily tested using our own prod.:

jp-tosca · 2024-01-23T19:49:23Z

I have right now some Datasets that have images and still don't show the thumbnail, see:

It is still not clear to me exactly why or how this happens, since we have a solution and identified these 2 bugs I thought it would be a good idea just to get rid of them and then come with a separate solution. This way we can start doing research and asses what would it take and why did the 3rd bug is happening.

Let's keep this PR, right now I will do some final QA on another PR and then come back to this and do some research. For my testing this not only happens when there is no image on the dataset.

landreev · 2024-01-23T20:03:39Z

Everything is happening too fast in that gif, lol 😄
But yeah, it does look like DS4 has BOTH image Datafiles, AND the custom logo (?)...
If it's really looking like that 3rd bug is NOT as straightforward as we thought it was, then that could be a reason to treat it as a separate issue.
But please do open an issue for it. Are you saying that you don't know for the fact how to reproduce/get to this condition that DS4 is in?

jp-tosca · 2024-01-23T20:37:25Z

That is correct as of now is not clear to me why or when the third bug happens. At first glance doesn't seem as straight forward as we initially thought and part of the reason why I thought on separating this from this issue. 😄

Sorry about the speed I will keep it in mind for next one. ⏩

landreev · 2024-01-23T21:33:31Z

@jp-tosca When you have a moment, could you check and post the output of ls -l dataset_logo* in the storage directory of the dataset DS4 above - the one where the custom thumbnail isn't showing?
I'm working on too many other things at once r/n, but very curious about it by now, so can't help myself.

jp-tosca · 2024-01-23T21:47:26Z

Sadly I deleted that installation already and I don't have it anymore since I am testing another PR and I needed to clear the environment multiple times but I will keep everyone updated tomorrow with my testing about this issue.

qqmyers · 2024-01-23T22:48:16Z

FWIW: It looks like

dataverse/src/main/java/edu/harvard/iq/dataverse/ThumbnailServiceWrapper.java

Lines 173 to 177 in 61f9d8a

    
           // We attempt to auto-select via the optimized, native query-based method 
        
           // from the DatasetVersionService: 
        
           if (datasetVersionService.getThumbnailByVersionId(versionId) == null) { 
        
               return null; 
        
           }

will return null (triggering use of the default icon) unless there is a file that is a valid thumbnail candidate. It looks like a check to see if there is a dataset-level logo uploaded is needed - probably makes sense to check before this since trying to scan the files for a thumb candidate could be heavyweight. So changing to something like: ```
boolean hasDatasetLogo = false;
StorageIO storageIO = null;
try {
storageIO = DataAccess.getStorageIO(dataset);
if (storageIO.isAuxObjectCached(DatasetUtil.datasetLogoFilenameFinal)) {
// If not, return null/use the default, otherwise pass the logo URL
hasDatasetLogo = true;
}
} catch (IOException ioex) {
logger.warning("getDatasetCardImageAsUrl(): Failed to initialize dataset StorageIO for "
+ dataset.getStorageIdentifier() + " (" + ioex.getMessage() + ")");
}
// If no other logo we attempt to auto-select via the optimized, native query-based method
// from the DatasetVersionService:
if (!hasDatasetLogo && datasetVersionService.getThumbnailByVersionId(versionId) == null) {
return null;
}

landreev · 2024-01-25T15:42:57Z

@jp-tosca To reiterate, if we are not fixing the other bug(s) in #10220 in this PR, please open a separate github issue for that asap. Otherwise we'll merge this PR and that will mark the issue as resolved without any trace left of the remaining problems.

jp-tosca · 2024-01-25T18:03:48Z

Just to follow up the Issue for the third bug was created and @qqmyers already submitted a PR for it 😃

github-actions · 2024-01-25T18:49:27Z

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:10220-thumbnail-size-bug

ghcr.io/gdcc/configbaker:10220-thumbnail-size-bug

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

The issue is partly fixed by the following commits, which are already part of the v6.2 release: IQSS#10269 IQSS#10258 Had to add "dataset.setUseGenericThumbnail(false);" line in the DatasetServiceBean to make it work.

Fix the issue with the thumbnail size

89b7f27

This comment has been minimized.

Sign in to view

qqmyers approved these changes Jan 23, 2024

View reviewed changes

landreev reviewed Jan 23, 2024

View reviewed changes

Merge branch 'develop' into 10220-thumbnail-size-bug

f206b4c

sekmiller self-assigned this Jan 24, 2024

This comment has been minimized.

Sign in to view

Merge branch 'develop' into 10220-thumbnail-size-bug

411f790

This comment has been minimized.

Sign in to view

jp-tosca mentioned this pull request Jan 25, 2024

Dataset thumbnail not applied in some cases #10268

Closed

Merge branch 'develop' into 10220-thumbnail-size-bug

108701c

sekmiller merged commit dada648 into develop Jan 25, 2024
19 checks passed

sekmiller deleted the 10220-thumbnail-size-bug branch January 25, 2024 19:31

pdurbin added this to the 6.2 milestone Jan 27, 2024

cmbz added the Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) label Jan 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the issue with the thumbnail size #10258

Fix the issue with the thumbnail size #10258

jp-tosca commented Jan 23, 2024

coveralls commented Jan 23, 2024 •

edited

Loading

This comment has been minimized.

qqmyers left a comment

landreev left a comment

landreev commented Jan 23, 2024

landreev commented Jan 23, 2024

jp-tosca commented Jan 23, 2024

landreev commented Jan 23, 2024 •

edited

Loading

jp-tosca commented Jan 23, 2024 •

edited

Loading

landreev commented Jan 23, 2024

jp-tosca commented Jan 23, 2024

qqmyers commented Jan 23, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

landreev commented Jan 25, 2024

jp-tosca commented Jan 25, 2024

github-actions bot commented Jan 25, 2024

Fix the issue with the thumbnail size #10258

Fix the issue with the thumbnail size #10258

Conversation

jp-tosca commented Jan 23, 2024

coveralls commented Jan 23, 2024 • edited Loading

This comment has been minimized.

qqmyers left a comment

Choose a reason for hiding this comment

landreev left a comment

Choose a reason for hiding this comment

landreev commented Jan 23, 2024

landreev commented Jan 23, 2024

jp-tosca commented Jan 23, 2024

landreev commented Jan 23, 2024 • edited Loading

jp-tosca commented Jan 23, 2024 • edited Loading

landreev commented Jan 23, 2024

jp-tosca commented Jan 23, 2024

qqmyers commented Jan 23, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

landreev commented Jan 25, 2024

jp-tosca commented Jan 25, 2024

github-actions bot commented Jan 25, 2024

coveralls commented Jan 23, 2024 •

edited

Loading

landreev commented Jan 23, 2024 •

edited

Loading

jp-tosca commented Jan 23, 2024 •

edited

Loading

qqmyers commented Jan 23, 2024 •

edited

Loading